Skip to content

Conversation

OrKoN
Copy link
Contributor

@OrKoN OrKoN commented Jan 16, 2025

Closes #858


Preview | Diff

@OrKoN
Copy link
Contributor Author

OrKoN commented Jan 16, 2025

I am not sure if accessing the navigation ID in this way is correct (it looked like it might not be for reload). Any suggestions are welcome.

@OrKoN OrKoN requested a review from sadym-chromium January 17, 2025 08:57
@OrKoN OrKoN requested a review from jgraham January 17, 2025 10:31
@OrKoN
Copy link
Contributor Author

OrKoN commented Jan 17, 2025

@jgraham what do you think about using ongoing navigation this way? PTAL

index.bs Outdated

1. Return the result of [=await a navigation=] with |navigable|, |request| and
|wait condition|.
1. Let |navigation id| be |navigable|'s [=ongoing navigation=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually doesn't work, and may require changes on the HTML side. The problem is that we can return from the above algorithm without starting a new navigation, and this step will get the id of some other navigation (or maybe null). Previously that was OK because we already knew the navigation id at this point, so in the next step we'd notice that a failure event had been dispatched and return. But now we need the navigation to be initiated to find the correct id.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the part using await for events after the navigate is also flawed, since by the time the await is called some events might have already happened?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looks like it probably is because await doesn't have a way to synchronously check if the resume condition was already met.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I am ready to propose a change to the HTML spec right now, but perhaps we could specify here that we want to yield the navigation id back to our algorithm as soon as it is defined on the step 6 and run the rest of the navigate/reload algorithm in parallel?

@OrKoN OrKoN force-pushed the orkon/align-navigation-with-html branch 2 times, most recently from 316f7d4 to 61f8f59 Compare January 17, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Navigate and reload calls are out of sync with the HTML spec?

3 participants